Hello @ajf58, and thank you very much for your first pull request to the Zephyr project!
Our Continuous Integration pipeline will execute a series of checks on your Pull Request commit messages and code, and you are expected to address any failures by updating the PR. Please take a look at our commit message guidelines to find out how to format your commit messages, and at our contribution workflow to understand how to update your Pull Request. If you haven't already, please make sure to review the project's Contributor Expectations and update (by amending and force-pushing the commits) your pull request if necessary.
If you are stuck or need help please join us on Discord and ask your question there. Additionally, you can escalate the review when applicable. 😊

This should be merged only after #62186 has been merged, so that it uses the new clock definitions

This should be merged only after #62186 has been merged, so that it uses the new clock definitions

Added the DNM label :)

Looks like #62186 has merged, can we keep this going?

Looks like #62186 has merged, can we keep this going?

Now that I've finished work for the year, yes. Locally I've rebased onto the HEAD of main to bring in the new clock controller work, and I think I've made the relevant other changes to my driver. I'll test those changes locally before pushing anything to my branch.

I think this is ready for another round of review feedback. This driver now includes both the time and the alarm API.

Testing

I've run this with the RTC API test suite, and got these results:

*** Booting Zephyr OS build zephyr-v3.5.0-3468-g897f8e5b481e ***
Running TESTSUITE rtc_api
===================================================================
START - test_alarm
I: Setting alarm
I: Setting alarm
I: Setting alarm
 PASS - test_alarm in 26.006 seconds
===================================================================
START - test_alarm_callback
I: Setting alarm
I: Setting alarm
 PASS - test_alarm_callback in 26.004 seconds
===================================================================
START - test_set_get_time
 PASS - test_set_get_time in 0.001 seconds
===================================================================
START - test_time_counting
 PASS - test_time_counting in 9.003 seconds
===================================================================
START - test_y2k

    Assertion failed at WEST_TOPDIR/zpico/tests/drivers/rtc/rtc_api/src/test_y2k.c:59: rtc_api_test_y2k: (rtm[Y2K].tm_sec not equal to SECONDS_AFTER)
wrong second: 2
 FAIL - test_y2k in 2.015 seconds
===================================================================
TESTSUITE rtc_api failed.

------ TESTSUITE SUMMARY START ------

SUITE FAIL -  80.00% [rtc_api]: pass = 4, fail = 1, skip = 0, total = 5 duration = 63.029 seconds
 - PASS - [rtc_api.test_alarm] duration = 26.006 seconds
 - PASS - [rtc_api.test_alarm_callback] duration = 26.004 seconds
 - PASS - [rtc_api.test_set_get_time] duration = 0.001 seconds
 - PASS - [rtc_api.test_time_counting] duration = 9.003 seconds
 - FAIL - [rtc_api.test_y2k] duration = 2.015 seconds

------ TESTSUITE SUMMARY END ------

===================================================================
PROJECT EXECUTION FAILED

the test_y2k failure is something I'm not sure about; I'm wondering if this test needs the same use of RTC_TEST_GET_SET_TIME_TOL as found elsewhere and I'd welcome some thoughts.

Nice, the rtc_utils are great :) I would like an extra test suite in tests/drivers/rtc/rtc_utils to validate the rtc_utils_validate_tm function on its own as well :) and I think the function rtc_utils_validate_tm should be named rtc_utils_validate_rtc_time since tm could refer to the struct tm which does allow -1 values :)

I would like an extra test suite in tests/drivers/rtc/rtc_utils to validate the rtc_utils_validate_tm function on its own as well :) and I think the function rtc_utils_validate_tm should be named rtc_utils_validate_rtc_time since tm could refer to the struct tm which does allow -1 values :)

I've done this as requested.

I did a quick correction and rebase to address a code-style issue. The CI pipeline failures are due to the runners disappearing:

The self-hosted runner: zephyr-runner-linux-x64-4xlarge-deployment-g2tcl-r7ttn lost communication with the server.

which I don't think is down to me adding a blank line in a source file ;-).

How do we proceed? Does this require someone with higher privileges to re-run the pipeline, for example?

Can we get this over the line please?

Hi @ajf58!
Congratulations on getting your very first Zephyr pull request merged 🎉🥳. This is a fantastic achievement, and we're thrilled to have you as part of our community!

To celebrate this milestone and showcase your contribution, we'd love to award you the Zephyr Technical Contributor badge. If you're interested, please claim your badge by filling out this form: Claim Your Zephyr Badge.

Thank you for your valuable input, and we look forward to seeing more of your contributions in the future! 🪁